Add executed MC/DC TestVectors to llvm-cov export
#105511
Conversation
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Swatinem
force-pushed
the
mcdc-export-testvectors
branch
from
5d05d99 to
6910802
Compare
Swatinem
force-pushed
the
mcdc-export-testvectors
branch
from
6910802 to
1f57500
Compare
|
@EugeneZelenko Hi! Can you please help to assign the right reviewers here? |
|
Thank you for implementing this! I wasn't aware of the outstanding issue and PR. Exporting to JSON was not an immediate need, so I appreciate the engagement in supporting the JSON more fully for MC/DC. I agree it makes sense right now to focus on executed test vectors. I will review this within the next few days. |
|
Sorry for the long delay in reviewing this code. Thank you for your contribution to improving this. In general, LGTM. However, can you also add a test to verify the JSON output? You can extend one of the existing test files. Also, it would be good to do a rebase given the age of the PR. Thanks! |
| } | ||
| } | ||
|
|
||
| json::Array gatherTestVectors(coverage::MCDCRecord &Record) { |
There was a problem hiding this comment.
Can you document your additions in the comment at the top of the file (line 23ff)?
|
@Swatinem @evodius96 I decided it's just easier to cherry pick since there are a tone of conflicts on rebabasing |
It adds a new JSON array with the list of test vectors to the end of the mcdc_records. I also bumped the json format version accordingly, which I believe wasn’t done properly in the past when new fields were added. This PR adds tests and comment docs for #105511 --------- Co-authored-by: Arpad Borsos <[email protected]>
It adds a new JSON array with the list of test vectors to the end of the mcdc_records. I also bumped the json format version accordingly, which I believe wasn’t done properly in the past when new fields were added. This PR adds tests and comment docs for llvm/llvm-project#105511 --------- Co-authored-by: Arpad Borsos <[email protected]>
|
Closing PR in deference to #159119 which is merged. Thanks! |
It adds a new JSON array with the list of test vectors to the end of the mcdc_records. I also bumped the json format version accordingly, which I believe wasn’t done properly in the past when new fields were added. This PR adds tests and comment docs for llvm#105511 --------- Co-authored-by: Arpad Borsos <[email protected]>
4 participants
This is my attempt at fixing #105489.
It adds a new JSON array with the list of test vectors to the end of the
mcdc_records.I also bumped the json format version accordingly, which I believe wasn’t done properly in the past when new fields were added.
Unfortunately, the
MCDCRecordonly contains the executed test vectors, not all the relevant ones.Still for forward compatibility, it still adds an
executed: truefield. So if in the future LLVM is able to output all relevant test vectors, one can emit those as well usingexecuted: false.I also had to resort to a
const_castbecause the it looks like thePosToIDmapping is mutated when reading from it 🙄. If I see it correctly, this shouldn’t pose a problem, as the only concurrency here seems to be related to processing multiple files in parallel, which should all have their independent copy of thisPosToIDmapping.